-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(zone): add Zone resource for Secure #609
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but it would be better to get some more experienced review
Computed: true, | ||
}, | ||
SchemaScopesKey: { | ||
Optional: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I don't know terraform well.
Is this resource related to the new platform
zones?
Because if it is, scopes
are mandatory, and they can be 1 or more.
On L:52 I see MaxItems:1
.
Also scopes ids are missing 🤔
This is an example response from staging:
// ....
"scopes": [
{
"id": 439653,
"rules": "",
"targetType": "azure"
},
{
"id": 439654,
"rules": "",
"targetType": "kubernetes"
},
{
"id": 439655,
"rules": "",
"targetType": "host"
}
]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for checking! yes, this resource is for creating new platform
zones, but since it's only for secure currently it is named like other secure resources.
I changed scopes
to be required field and also I added id
as a field for scope
... nice catch! 👍
MaxItems:1 is for "scopes" field - we can have only one of those. MinItems: 1 is set for "scope", because we need at least one of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed scopes
to simplify the schema
No description provided.